Skip to content

Conversation

@HuijungYoon
Copy link

When using let? Some(x) = None in a function returning option<T>, the early return was incorrectly returning a variable instead of None.

This fixes the issue by explicitly returning None/Some values instead of using pattern-matched variables, ensuring correct type propagation and JS interop compatibility.

Fixes #8085

@fhammerschmidt fhammerschmidt requested a review from zth December 14, 2025 13:31
@nojaf
Copy link
Member

nojaf commented Dec 18, 2025

@HuijungYoon could you please run make format?

Copy link
Member

@zth zth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @HuijungYoon , thank you for taking this on!

Returning a variable is intentional (it means we don't need to recreate the value at runtime). So that should not be changed. The problem to fix is what this variable is called.

So, the fix here should most likely be to propagate the name of the variable used in the let binding:

let? Some(myVar) = someOptFn()
// We need to propagate the `myVar` name to where `x` is now used hard coded

When using `let? Some(myVar) = ...`, the variable name was hardcoded as "x"
instead of using the actual pattern variable name "myVar".

This fix extracts the variable name from the pattern and uses it in the
early return cases, ensuring proper variable propagation and avoiding
unnecessary runtime allocations.

Fixes rescript-lang#8085

Signed-Off-By: [Huijung Yoon] <[markup3604@gmail.com]>
@HuijungYoon HuijungYoon force-pushed the fix-let-unwrap-option-early-return branch from 23985b2 to 7e70c3a Compare December 18, 2025 11:49
@HuijungYoon
Copy link
Author

@nojaf Ok i did.
@zth thanks you for your review. as your instruction.
i propagated the name of the variable used in the let binding.
could you review code again when you have a time?

@zth
Copy link
Member

zth commented Dec 18, 2025

Thank you. Now you need to do these things:

  • Run make format and make tests
  • Add a changelog entry

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

let? ignores variable name in generated javascript

3 participants